bun:test: restore a spy when its object refuses to delete it - #48071

Repro

import { test, spyOn } from "bun:test";
class Svc { method() { return "original"; } }
test("t", () => {
  const o = new Svc();
  const s = spyOn(o, "method").mockReturnValue("spied");
  Object.freeze(o); // or Object.seal(o), or Object.defineProperty(o, "method", { configurable: false })
  s.mockRestore(); // or mock.restore(), restoreMocks, `using`, or the end of the test file
  console.log(o.method(), Object.hasOwn(o, "method")); // main: undefined true. This PR: original false
});

Decisions to check

1. A plain object that is locked after spyOn() is restored through the lock. The own-property restore does that already, and bun test: Vitest compatibility #44833 tests it ("restoring a native accessor that was made non-configurable meanwhile").
2. The direct store replaced VM::DeletePropertyModeScope(IgnoreConfigurable) from the first draft. The store is putDirect, as in the own-property restore. It needs no VM-wide mode, and it also empties a field that compiled code lays out, so bun test and bun test --aot agree.
3. The restore removes whatever own property is on the key, as an unlocked restore does. That includes a non-configurable property that user code put there. 1.3.13 to 1.4.3 overwrote it with the original. Main keeps it only because its delete is refused. A check for the spy before the direct store was tried. It breaks stacked spies (spyOn(o, k), spyOn(o, k, "get"), freeze, restore the first one): the first spy leaves, and the restore of the second puts the first mock back for good.
4. A Proxy that keeps the spy gets the pre-spy value as its own property. Two other repairs were weighed. A delete on the target behind the traps is exact for Vue's readonly(), but it writes where the traps do not see it and leaves happy-dom with the unbound prototype method. A spy left in place as a pass-through still answers isMockFunction and still records.
5. The spy is looked for on the target behind the Proxy, with no trap, and the answer of a deleteProperty or defineProperty trap is not read. happy-dom hides every key of its target from getOwnPropertyDescriptor. Vue's readonly() answers true to a delete that it ignores. A Proxy that keeps the spy somewhere else than on its target, or that stores a wrapper of it, is not seen: the spy stays with no error, as on main. No library with that shape was found, and a read through the traps would add a trap call to each restore through a Proxy.
6. The TypeError is the same for every flavour. Vitest 5.0.3 drops the result of Reflect.deleteProperty and leaves the spy. Jest 30.5.2 throws. If vi spies must stay silent, it is one isVitest check before the throw.
7. The sentence is the one of spyOn: install and restore through one writer that throws when the object refuses #34976, so a refused restore has one message. Review it as code.
8. A kept spy is reported at the end of the file only while its object is alive, because the target is held weakly. This PR does not change that. bun:test: hold the spyOn target and property key as GC references so a collected spy leaks nothing #40382 is about that reference. One more effect of it, on main too: a spy through a Proxy that nothing else references is not restored after the Proxy is collected, and it stays on the object behind the Proxy.
9. A Proxy gets no direct store, also not on the object behind it. spyOn() through a Proxy over an object that is already frozen throws at install, so the restore does not go where the install cannot. A Proxy with no traps over an object that is frozen after spyOn() gets the TypeError.
10. A kept spy stays in the list and is retried, which is the rule of main for each restore that throws (when restoring one spy throws, the others are restored and that one is retried next time). The retry restores it when the object allows it again. Jest 30.5.2 repeats the error too: jest.restoreAllMocks() throws in each afterEach (4 of 4), and then the file fails as a whole.

What changes against main

Measured (release builds of main b1305dd and of this PR, bun run build:release, same machine)

• Instructions (gdb stepi per symbol, pairs 41 to 45 of 50, a fresh object per pair, 3 runs): putBackOriginal 56 to 63 for an inherited-method spy, 59 to 62 for an own-method spy. clearSpy 39 to 39. mockRestore() in total 2,101 to 2,108 and 431 to 435. spyOn 433 to 438 (inherited) and 408 to 410 (own).
• .text 110,627,497 to 110,629,289 bytes (+1,792, llvm-size -A). putBackOriginal 585 to 895 B. New cold takeSpyOffOrThrow 1,182 B. spyOn +114 B. Three small out-of-line helpers, 372 B. putOwnPropertyStorage -10 B. No other symbol of 163,450 changed (llvm-nm -S).
• VM::m_deletePropertyMode (gdb awatch): 0 writes per restore on both builds, also for an object frozen after spyOn().
• Object frozen after spyOn(): 2 JSObject::deleteProperty calls per restore (main: 1, refused). 10,000 frozen instances, one spy each, mock.restore(): +13 Structures (control with no freeze: +10,000, on main too).
• GC cells per spyOn() + mockRestore() pair on an unlocked object: equal on both builds, type by type (bun:jsc heapStats, 500 pairs).
• Proxy traps per mockRestore(): deleteProperty 1 when the trap deletes, as on main. A kept spy adds defineProperty 1.
• 188 kinds of objects, 3 locks, string and symbol keys, value, get and set spies: main leaves the spy on 27,514 of 29,631 locked rows. This PR restores 29,301 of 29,760. The rest: 135 rows of a node:vm global, where the lock itself defines the key on the sandbox (the same on main), and Proxy rows that are locked through the proxy (TypeError).
• bun test --aot: 147 of 147 locked rows on classes with compiled fields are restored, as without the flag. The first draft threw for 78 of them.
• One kept spy (the element of a frozen array): two files and no restore call: 1 error line under the file that made the spy, exit 1. restoreMocks = true and 3 later tests: 3 fail. afterEach(() => mock.restore()) and 4 later tests: 5 fail. Main prints the same counts for a Proxy trap that throws. An object frozen after spyOn(): 0 fail, 0 error lines, exit 0.
• Debug build with BUN_JSC_validateExceptionChecks=1 over mock-fn.test.js and the probe files: 0 unchecked exceptions.
• Each clause of the change was removed in turn, 15 builds. At least one test fails each time. The unread answer of the defineProperty trap came later: its two tests fail on the build before it.

References, run here

• Jest 30.5.2: the restore is delete object[methodKey] in strict mode. A frozen object: TypeError: Cannot delete property 'method' of [object Object], the spy stays, and the suite fails at teardown. A Proxy that refuses: TypeError: 'deleteProperty' on proxy: trap returned falsish for property 'method'.
• Vitest 5.0.3: the restore is Reflect.deleteProperty(object, key) with the result dropped. No error, the spy stays. After mockRestore() it calls the original. After vi.restoreAllMocks() it still returns the mocked value.

Self-review

12 concerns were raised, 10 are addressed. Two are left for a maintainer: the vi rule (decision 6) and the lifetime of the report (decision 8).

The same pattern at other sites, not changed here

• src/jsc/bindings/ViUtils.cpp:91 and :179 (vi.unstubAllGlobals(), vi.unstubAllEnvs()): a stub of a new global that is made non-configurable stays. A separate change is in progress.
• src/js/internal/test/globals.ts:122 and :128: the cleanup of built-in globals between test files. It runs inside try {} catch {} and compares shapes again afterwards.
• src/jsc/bindings/node/NodeTimers.cpp:467 and src/runtime/test_runner/timers/FakeTimers.rs:859: they delete the clock marker that Bun puts on setTimeout.
• The results of overrideExportValue() and tryReplaceExport() in putBackOriginal are not read.
• An own method that is restored on an object frozen after spyOn() comes back writable and configurable (old, also in 1.4.3).
• Two spies that both stay at the end of a file: one error is reported. A separate change is in progress.
• vi.mockObject(value, { spy: true }) is not restored at the end of a file. A separate change is in progress.

Related PRs

• spyOn: install and restore through one writer that throws when the object refuses #34976 (open, in conflict with main): its refused-restore sentence and its rule that a refused spy stays registered are used here. Its install half is not in this PR.
• bun:test: store a spy on a native custom value the way an assignment does #47883 (open): it edits the accessor arm of putBackOriginal next to this change. The PR that lands second has a small merge.

Not run: macOS and Windows. On Windows process.env is a Proxy, so a spy on it takes the Proxy path of this change.